This PR modifies tests/ui/issues/. If this PR is adding new tests to tests/ui/issues/,
please refrain from doing so, and instead add it to more descriptive subdirectories.

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@davidtwco

Thank you @samueltardieu for your feedback. Regarding the small changes to unrelated files, I believe that was my auto-formatter doing me a disservice, and I believe I have removed said changes from the commits.
I also believe I have addressed the other points you made, but do let me know if anything is amiss.
Thanks again.

As far as I can tell those files have already been removed, let me know if I'm missing anything. Thank you

needless_maybe_sized.stderr
needless_maybe_sized.rs

I think you still have to remove src/tools/clippy/tests/ui/needless_maybe_sized.rs and src/tools/clippy/tests/ui/needless_maybe_sized.stderr.

These have been removed as far as I can tell, they're just showing as moves in Git as similar tests have been added to the rustc ui test suite w/ some changes.

I think you still have to remove src/tools/clippy/tests/ui/needless_maybe_sized.rs and src/tools/clippy/tests/ui/needless_maybe_sized.stderr.

These have been removed as far as I can tell, they're just showing as moves in Git as similar tests have been added to the rustc ui test suite w/ some changes.

Oh yes, the subpar GitHub diff UI shows this only at the point of destination, not at the point of origin.

I have added a .fixed file to the tests

@rfcbot fcp merge

maybe_sized_and_sized_supertrait or overridden_maybe_sized.

For the other, redundant_sized_bounds or, if more general, bounds_implied_by_supertraits or redundant_bounds.

Those are good points. And it seems we may have been unclear on what's included in this PR. Let's cancel the FCP for the moment. We'll then think about what we want here and repropose FCP.

@rfcbot fcp cancel

@traviscross proposal cancelled.

Do we need to keep starting and stopping FCPs for small naming changes? This would be the third FCP that will be accepted for this patch, with one cancelled after two days because of the addition of a plural and another after waiting the full ten days.

We're happy to name this whatever t-lang would like or split it into two lints, all we need is some clarity on what t-lang wants, ideally without having to wait another ten days for something that hasn't been at all controversial.

Sorry about the noise. This is following our normal process for what to do in this circumstance, but applied to a small thing like this, it can of course look pretty silly. The purpose of the process is to make sure it's clear what we've agreed to exactly. I'll try to make sure we get it right in the next round.

In the meeting, we had ended up a bit confused about what this lint as proposed in this PR does exactly.

The documentation for it says this:

declare_lint! {
    /// The `redundant_sizedness_bounds` lint detects redundant sizedness bounds
    /// applied to type parameters that are already otherwise implied.
    ///
    /// ### Example
    ///
    /// ```rust
    /// // `T` must be `Sized` due to the bound `Clone`, thus `?Sized` is redundant.
    /// fn f<T: Clone + ?Sized>(t: &T) {}
    /// // `T` is `Sized` due to `Default` bound, thus the explicit `Sized` bound is redundant.
    /// fn g<T: Default + Sized>(t: &T) {}
    /// ```
    ///
    /// {{produces}}
    ///
    /// ### Explanation
    ///
    /// Sizedness bounds that have no effect, as another bound implies `Sized`,
    /// are redundant and can be misleading. This lint notifies the user of
    /// these redundant bounds.
    pub REDUNDANT_SIZEDNESS_BOUNDS,
    Warn,
    "a sizedness bound that is redundant due to another bound"
}

That it, the documentation says it lints both against a relaxation of the default Sized bound (?Sized) that can't have any effect and a superfluous Sized bound that's already implied by a supertrait relationship.

However, in the meeting, in looking at the tests, we couldn't find any examples of linting against a Sized bound that's already implied.

In the code,

fn check_redundant_sizedness_bounds(
    redundant_bound: DefId,
    redundant_bound_polarity: BoundPolarity,
    // ...

I notice that it takes a polarity, so it seems designed to go both ways. However, in testing the branch, I can confirm that it does not lint against, e.g., fn f<T: Default + Sized>() {}.

What exactly was intended to be proposed here? Was the idea perhaps that this lint was to be expanded in the future?

If we were to lint redundant Sized bounds, was the idea that we'd only lint when the bound was redundant due to its presence as a supertrait or that we'd also lint when it was redundant due to being a default bound (e.g. fn f<T: Sized + 'a>() {})?

In the meeting, we talked about how perhaps it would make sense to have a more general lint that fires when any trait appears in a list of bounds that is already implied due to being a supertrait. In that world, it might be a bit awkward to have a separate lint that did that just for Sized. Maybe there are three underlying lints here:

1. A lint against a trait appearing in a bounds list when it is already implied due to being a supertrait (recursively) of another trait in the bounds list.
2. A relaxation of a default bound that can't have effect due to another bound.
3. A bound that is redundant due to it already being a default bound.

Thoughts on that? Would something like that achieve the goal here?

However, in the meeting, in looking at the tests, we couldn't find any examples of linting against a Sized bound that's already implied.

In the code,

fn check_redundant_sizedness_bounds(
    redundant_bound: DefId,
    redundant_bound_polarity: BoundPolarity,
    // ...

I notice that it takes a polarity, so it seems designed to go both ways. However, in testing the branch, I can confirm that it does not lint against, e.g., fn f<T: Default + Sized>() {}.

The polarity is only mentioned because that's how ?Sized is represented in rustc (as BoundPolarity::Maybe).

The Default + Sized example is just incorrect, the lint shouldn't trigger on that and wasn't intended to. I think this was changed in an incorrect attempt to respond to your comment here - #145924 (comment).

What exactly was intended to be proposed here? Was the idea perhaps that this lint was to be expanded in the future?

Just uplifting of the existing redundant ?Sized lint (e.g. Clone + ?Sized), with additional support for the Sized Hierarchy traits (e.g. Clone + MetaSized). No intentions to expand it further than that.

If we were to lint redundant Sized bounds, was the idea that we'd only lint when the bound was redundant due to its presence as a supertrait or that we'd also lint when it was redundant due to being a default bound (e.g. fn f<T: Sized + 'a>() {})?

Just the former.

In the meeting, we talked about how perhaps it would make sense to have a more general lint that fires when any trait appears in a list of bounds that is already implied due to being a supertrait. In that world, it might be a bit awkward to have a separate lint that did that just for Sized. Maybe there are three underlying lints here:

1. A lint against a trait appearing in a bounds list when it is already implied due to being a supertrait (recursively) of another trait in the bounds list.
2. A relaxation of a default bound that can't have effect due to another bound.
3. A bound that is redundant due to it already being a default bound.

Thoughts on that? Would something like that achieve the goal here?

I'm open to making it more general, subject to perf being okay (it could be an awful lot of querying supertraits of supertraits of traits, etc).

Thanks for that. OK, so the case we want to address right now, then, is:

2. A relaxation of a default bound that can't have effect due to another bound.

Probably I'd think to call this lint unused_relaxed_bounds.

Why "unused"?

In saying ?Sized, we're trying to relax a default bound, but that relaxation isn't actually used. This reminds me of, e.g. unused_associated_type_bounds:

trait Tr {
    type Ty where Self: Sized;
}
type Alias = dyn Tr<Ty = ()>;
//~^ warning: unnecessary associated type bound for dyn-incompatible
//~|          associated type
//~| note: this associated type has a `where Self: Sized` bound, and
//~|       while the associated type can be specified, it cannot be
//~|       used because trait objects are never `Sized`

I.e., there's a Sized bound elsewhere causing this bound to be "unused".

It also reminds me of unused_assignments:

fn f() -> u8 {
    let mut x;
    x = 1;
    //~^ warning: value assigned to `x` is never read
    //~| note: maybe it is overwritten before being read?
    x = 2;
    x
}

I.e., we "overrode" the value of x, so the original assignment is "unused".

Similarly, unused_comparisons:

fn foo(x: u8) {
    if x < 0 {
    //~^ warning: comparison is useless due to type limits
        engage_safety();
    }
}

I.e., the comparison has semantic meaning but is "useless". In the same way, saying T: ?Sized + Default has an obvious semantic meaning, "first, relax the default Sized bound, then add a Default bound which implies Sized", but it renders that first part useless and therefore "unused".

More broadly, this fits the model for me of what we lint as "unused", which are "things that have obvious semantic meaning but are useless, and which should all be deleted after turning off #![allow(unused)] before committing the change".

Why "relaxed bounds"?

The core idea of this lint is that the user is trying to relax a bound but that, due to another bound, this relaxation isn't applied (or "used") by the compiler. While it happens to be true that the only bound that can be relaxed today in Rust is the default Sized bound, if we ever had another such bound that could be relaxed, I'd want this same lint to apply, and even if we never do that, in my mind (as a user of the lint), the idea of this lint has more to do with default bounds and when they can be relaxed or not than it has to do with Sized.

Why not "maybe"?

I don't think we should use the word "maybe" to refer to "?X" bound relaxations. Either there's a predicate stating that a type parameter implements some trait or there isn't. While of course I get it -- the type argument provided may or may not implement the trait -- I just think "maybe" is speaking to the wrong thing here. It focuses on the type argument when what makes more sense to focus on is the type parameter. And the type parameter is not in a "maybe" state.

Does unused_relaxed_bounds seem OK to people for this lint? If so, we'll scope this PR to down to this, and I'll proposed FCP merge on it with this name.

I don't want to bikeshed: I like this lint, and I'd rather have the better version of this lint by any name sooner, than a perfectly-named lint later. But I'm currently working on making cargo-semver-checks detect the SemVer breakage analogue of this lint, and I have a concern about "unused".

I think it's at least a nice-to-have if related SemVer ("across time") and rustc/clippy ("single point in time") lints have similar names. While "unused" seems plausibly reasonable in the "point in time" setting, it would imply a "becomes unused" lint in the SemVer setting. That doesn't seem like the right word at all to me.

An example of the SemVer lint would catch e.g. a T: ?Sized + Foo where Foo in a later version starts implying Sized, thereby constraining T as well even though there was no syntactic change near T itself. The word "unused" usually appears in contexts like "unused import" and completely fails to communicate the seriousness of such a situation. If cargo-semver-checks tells the user: "your T now has an unused ?Sized bound," I'd be shocked if that made them realize they are about to make a major breaking change!

I think I'd rather say that ?Sized is inapplicable, or inaccurate, or that it's non-binding (as in, it fails to correctly describe the actual bounds of the API). I'm bad at naming things, and there are probably better words too. But "unused" doesn't feel like the right choice to me.

I'm open to making it more general, subject to perf being okay (it could be an awful lot of querying supertraits of supertraits of traits, etc).

I'm in the middle of prototyping something that does that kind of awful lot of querying, for rustdoc JSON and cargo-semver-checks: #143197 Maybe we should talk? :)

An example of the SemVer lint would catch e.g. a T: ?Sized + Foo where Foo in a later version starts implying Sized, thereby constraining T...

If you could elaborate this example, that'd be helpful. In particular, why are we worried about the bound here rather than the change to the trait?

☔ The latest upstream changes (presumably #147779) made this pull request unmergeable. Please resolve the merge conflicts.

I think the concern is that you may unknowingly start disallowing unsized types in your API when you intended to prevent that with ?Sized. So the severity of the lint is higher than a dead code lint. I don't know if we can actually do much better for the name. But maybe "reversed" or "overridden" or "negated" or "invalid" or "noop".

I think the concern is that you may unknowingly start disallowing unsized types in your API when you intended to prevent that with ?Sized. So the severity of the lint is higher than a dead code lint.

This is exactly it.

I'm concerned the user will see a rustc/clippy lints for needless_maybe_sized and go "ehh, not that big of a deal" and possibly #[allow] it. Then they'll see a cargo-semver-checks lint for became_needlessly_maybe_sized and go "still not a big deal." Then they'll ship SemVer breakage.

I like all names @camsteffen and I proposed better than "unused" because they signal a higher severity. I'd still avoid "noop" if possible because I think it has the same risk, just not as much of it.

It's still not coming into focus for me. Is this the concern?

If the user has, in v1.1,

pub fn f<T: ?Sized>(_: &dyn Deref<Target = T>) {}

and then the user tries to ship, in v1.2,

pub trait Tr: Sized {}
impl<T> Tr for T {}
pub fn f<T: ?Sized + Tr>(_: &dyn Deref<Target = T>) {}

getting this warning,

warning: `?Sized` has no effect due to another bound
  --> src/main.rs
   |
   | pub fn f<T: ?Sized + Tr>(_: &dyn Deref<Target = T>) {}
   |             ^^^^^^ help: remove this unused bound relaxation
   |
   = note: generic parameters are assumed to be `Sized` unless
           the bound is relaxed by writing `?Sized`, but here
           this has no effect as another bound implies `Sized`
   = note: `#[warn(unused_relaxed_bounds)]` on by default
note: the bound that implies `Sized` is here
  --> src/main.rs
   |
   | pub fn f<T: ?Sized + Tr>(_: &dyn Deref<Target = T>) {}
   |                      ^^ note: this bound implies `Sized`
   |
note: `Sized` is implied by `Tr` as `Sized` is a supertrait
  --> src/main.rs
   |
   | pub trait Tr: Sized {}
   |               ^^^^^ note: `Sized` is a supertrait
   |

then the idea is that the user is going to reason, "I added a new T: Tr bound on the public function f, which would normally be breaking, but it's my trait, and I've implemented it for all T, so this should be OK. Rust is warning me that the ?Sized bound relaxation, which I put there to ensure that types that don't implement Sized are accepted, is now unused and is being ignored by the compiler when I wasn't getting that before, but that's probably OK because...? Well, I mean, Rust even suggested deleting the ?Sized, so it must be OK to delete, right?"

And then cargo-semver-check is going to report this as SemVer breakage, and the user will think, "no, it's OK, Rust told me that ?Sized was unused and suggested I delete the bound relaxation, so this must be OK."

Is that right, or no?

As context, I take for granted that unused warnings in Rust must be taken seriously.

E.g., if we have,

pub trait Tr: Sized { fn f(self) -> u8 { 1 } }
impl<T> Tr for T {}

and then somewhere else we write,

pub struct W<T>(T);

impl<T> W<T> {
    fn f(&self) -> u8 { 2 }
}

fn main() {
    println!("{}", W(()).f());
}

thinking that, "obviously this should print 2; W<T>::f is an inherent method, and inherent methods take priority", or maybe just not noticing that trait or trait impl at all, and then we get,

warning: method `f` is never used
 --> src/main.rs:7:8
  |
6 | impl<T> W<T> {
  | ------------ method in this implementation
7 |     fn f(&self) -> u8 { 2 }
  |        ^
  |
  = note: `#[warn(dead_code)]` (part of `#[warn(unused)]`) on by default

that's a very serious warning. That causes me to sit up in my chair. I think, "if that code isn't being used, then what am I running? Not what I was expecting to run, obviously."

Any unused warning that I wasn't expecting does that for me, for this kind of reason. And so it's what I think about, in this case, when I think about how I'd react.

One ignores unexpected unused warnings in Rust at one's peril.

I don't really object to unused personally. I agree whatever lint should make you stop and consider. However, there is an unused lint group for lints with a certain semantic and this lint wouldn't fit in that group. Unused typically means "you wrote X and X is unreachable" rather than "you wrote code X which looks like it does Y but it actually does not".

Responding to your first example, FWIW, a more subtle story would be that you have T: Tr + ?Sized and there is no lint. You then add a supertrait Foo to Tr and Foo is Sized.

Responding to your first example, FWIW, a more subtle story would be that you have T: Tr + ?Sized and there is no lint. You then add a supertrait Foo to Tr and Foo is Sized.

You've then made a breaking change to the trait. At that point, it seems like the use-site bound is the least of one's SemVer problems.

I agree whatever lint should make you stop and consider. However, there is an unused lint group for lints with a certain semantic and this lint wouldn't fit in that group. Unused typically means "you wrote X and X is unreachable" rather than "you wrote code X which looks like it does Y but it actually does not".

Strongly agreed with this. "Users should take lints seriously" I agree with, but that is not the same statement as "users take lints seriously" which I don't think follows or is accurate in general.

A great strength of Rust is setting up users to fall into the pit of success. Using unused in this lint does not feel like a pit of success choice.

We should also remember there are other analogous flavors of "you wrote code X which looks like it does Y but it actually does not." What we name this lint will set a precedent for name those future lints. One example with 'static bounds: playground

trait Tr: 'static {}

#[allow(private_bounds)]
pub trait Intermediate: Tr {}

pub fn f<'a, T: Intermediate + 'a>(_: &'a str, _: &dyn Deref<Target = T>) {}

Here, T: 'static implicitly because of T -> Intermediate -> Tr -> 'static but:

• The source code for f() makes it look like it only needs to be 'a.
• Rustdoc and docs.rs for f() make it look like it only needs to be 'a`.
• The same is true for both source code and rustdoc/docs.rs for Intermediate---neither mentions 'static.

This does not currently trigger any lint, neither in rustc nor in clippy. I believe it should. By precedent from here, would such a lint then be named unused_lifetime_bound?

If so, IMHO we'll end up dramatically redefining what the unused group of lints does. We'll do so in a step-by-step fashion, one lint at a time, which users will not notice until they get burned one day because their prior conditioning about the meaning and importance of unused lints.

Not a pit of success choice.

You've then made a breaking change to the trait. At that point, it seems like the use-site bound is the least of one's SemVer problems.

I agree, the breaking change is to the trait. But that can be viciously difficult to catch, and I believe we should offer as many opportunities as possible for users to spot and stop this problem. Consider this example: playground

trait Tr {}

#[allow(private_bounds)]
pub trait Intermediate: Tr {}

pub fn f<T: ?Sized + Intermediate>(_: &dyn Deref<Target = T>) {}

Let's assume there are suitable impls to allow downstream users to have access to both sized and unsized types that f() is callable with.

Furthermore, assume Tr, Intermediate, and f are all in separate files. For example, maybe Tr is in a mod not pub mod and is surrounded by other non-pub items.

Say one then changes Tr to trait Tr: Sized {}. This is undoubtedly a major breaking change, but one that's viciously difficult to notice---IMHO analogous to Send/Sync auto-trait changes:

• It's caused by a change to a non-pub item.
• The breakage happens far from the place where the change happened.
• It can happen through arbitrarily many layers of indirection, not just the one layer we have here.
• It's infectious: if one crate breaks SemVer this way, downstream crates depending on the same trait bound tree will also break---and in a way that's easy to misinterpret as the downstream crate breaking SemVer. To the users of downstream crates, it'll appear that post-cargo update there's now a bound-related compile error at the call site to the downstream crate, which will cause additional support and triage burden etc.

It's thus all too easy for a trait bound change on a private trait, in an entirely non-pub corner of one's crate, to escalate and ripple across the ecosystem.

SemVer breakage is a numbers game. IMHO we should play the numbers by doing our best to block that escalation path in as many places as possible. I believe this includes choosing a name for this lint that makes it harder to (mistakenly) ignore and just suppress away.

To see how users might not understand the broader context of a lint and suppress it without due care, we only have to look as far as that #[allow(private_bounds)]. To illustrate my point (i.e. not proposing we do this) imagine if that line instead said #[allow(all_private_traits_and_their_bounds_recursively_are_forever_public_api_here)] --- I think that would have also contributed to blocking this kind of escalation path.

I'm happy to answer questions and generally engage with the discussion, but I don't want to do so at the cost of shipping this lint at all. As I mentioned, my goal is not to bikeshed this :)

I feel I've made my case, and I hope you've found it persuasive. If not, that's fine! Consider my voice heard and, as far as I'm concerned, feel free to proceed!

Thanks for those details and discussion. To answer your question, given this:

pub trait Tr: 'static {}
pub fn f<'a, T: Tr + 'a>() {}
//                   ^^ help: remove this bound

Yes, I would suggest that bound is unused. It can be removed from the program without effect; the compiler is free to ignore it. While one could equally describe it as "ineffectual", "pointless", "useless", "redundant", "superfluous", "needless", "inconsequential", "unnecessary", etc., these all seem like synonyms for unused in a Rust context. Other unused_* lints could equally use these synonyms.

In some cases where we use "unused" today, one could even argue for stronger terms like "contradictory" or "conflicting". E.g.:

#[unsafe(export_name = "foo")] //~ WARNING: unused attribute
#[unsafe(export_name = "bar")]
//~^ This could be made more subtle with `cfg_attr`, macros, etc.
pub fn f() {}

Here, conflicting_* would be a reasonable name (similar to conflicting_repr_hints). Unlike with ?Sized + Sized, there is no semantically obvious way to combine these. That's why "conflicting" feels appropriate, to me, in the above, but not for ?Sized + Sized.

Broadly, I think the purpose of our lint names is to be as precise, concise, and consistent as possible, not to try to hit a particular scariness level. If we want to scare users, the lint output seems a better place to do that than the name.

All that said, if in discussion we turn out to not be happy with "unused", here are some other names that have occurred to me:

• subsumed_relaxed_bounds: The relaxed bound has been "subsumed" by a tighter bound, just as 'a is "subsumed" by 'static. There's maybe some tension in the relaxed_bounds case due to relaxation going in one direction from the default and then another bound going in the other direction, but we should probably consider this in the context of our "ask for what you need" direction in the hierarchy of Sized types work. I.e., the bound will be something like T: Pointee + Tr where trait Tr: MetaSized {}, and here it will be true that Pointee is subsumed by MetaSized.
• negated_relaxed_bounds: This hints specifically at the idea that the relaxed bound has been made ineffective.
• misleading_relaxed_bounds: This one I toyed with, but I don't really care for it, as it seems to just push the question down a level. "Why is it misleading? Because it has no effect." It seems better to try to speak to what's happening in the compiler, where possible, rather than to the speculated effect on the user's state of mind.

The "ask for what you need" hierarchy of Sized traits model does raise another question here for us. If we did have a separate lint for when a trait in a bound is already implied by being a supertrait of another trait in a bound, would we expect that to additionally fire given T: Pointee + Tr where Tr: MetaSized? It'd both be true that the bound relaxation has no effect and that a trait in the bound is already implied.

That's something to think about. Maybe, given our direction here, this new lint could itself be "subsumed".

Broadly, I think the purpose of our lint names is to be as precise, concise, and consistent as possible, not to try to hit a particular scariness level. If we want to scare users, the lint output seems a better place to do that than the name.

Ordinarily I would agree with this. But naming a lint unused_* to me implies that it also becomes part of the unused lint group, which is where the real problem lies IMHO. Every other lint in there is genuinely a fairly trivial concern, quite unlike the hazards that this issue is about.

We don't want #[allow(unused)] that was intended to en masse suppress a group of other lints to cause ineffective and confusing ?Sized bounds to remain unnoticed.

That's something to think about. Maybe, given our direction here, this new lint could itself be "subsumed".

I'm happy with "subsumed" and it genuinely feels like the best name of the lot. It could also be the name of a new lint group for all these lints, which I think would be a good outcome.

#[unsafe(export_name = "foo")] //~ WARNING: unused attribute
#[unsafe(export_name = "bar")]

That's a good example. I feel convinced now that unused is actually not just for "trivial" concerns and may include cases of two things conflicting with each other and one wins out.

Given that it sounds like it was news to both of us that an ineffective unsafe is a lint in the unused group, I feel more convinced that that lint should also be renamed.

We discussed this in the lang-team meeting and concluded two things

• we all agree that T: ?Sized + Clone should lint, as the ?Sized is misleading
• but we were split on whether T: Pointee + Clone should lint.

In particular, other kinds of redundancy are not clearly worth linting over, for example, given trait Foo: Bar, T: Foo + Bar might be a problem, but maybe not, maybe the author wants to emphasize that they directly use methods from Bar as well as Foo.

The "double role" of Pointee here makes this case a bit subtle.

We were tempted to say: let's just do it for ?Sized, but @traviscross pointed out that the motivation of this PR is likely more to do with the second case, which raised the question: How important is this particular change to the overall hierarchy of sized traits? It seems like at most a "nice to have" and not essential, but maybe there is a connection we are not understanding.

Can somebody update as to this question so we can review again in next triage? Thanks.

Definitely a fan of marking bounds that "don't work", in some form.

That said, I'd trying to think of whether there's potentially any reasons that you might want to write something for explicitness even if it's arguably useless. Like T: SomeForeignTrait + Clone doesn't seem unreasonable to write, even if Clone is a supertrait of SomeForeignTrait. Because even if it's semver breaking to remove the supertrait, if I'm intentionally cloning it I might want to just have that there to see more easily in the code and/or the rustdoc. And if the crate does do a breaking change to remove a supertrait, it might be nice that when you do the major version bump that you still had the "unused" bound.

So I'm a fan of linting on the "conflict"-y cases like T: Clone + ?Sized where they're in a sense contradicting each other. Someone mentioned (forget who, sorry) that if we had negative bounds on negative impls, then T: Foo + !Foo is clearly worth linting.

As a result I'd be fine focusing to the ?Sized cases in this PR, and figure out some of the other more complicated cases (like whether we can meaningfully distinguish T: SomeForeignTrait + Clone from T: Copy + Clone) later in a different lint.

For my part, I'm not sure I would be happy with linting on T: ?Sized + Clone if we weren't happy with linting on T: Pointee + Clone, and I'm not sure I'd be happy with linting on T: Pointee + Clone if we weren't happy with linting on T: Sized + Copy (where Copy: const Sized) or more generally on trait bounds that are unused due to being already implied by being a supertrait of another trait in the bound. From a user pointer of view, I'm not sure these will all really seem that different.

At the same time, I'm sympathetic to @scottmcm's point that writing out the subtrait explicitly in the bound can sometimes be desired to improve local reasoning or (as I'd maybe see it) as a static assertion.